feat: surface 402 payment recovery info so interrupted payments are never paid twice#59
Conversation
…ever paid twice Fixes #55 and #58 by updating @getalby/lightning-tools to v9.0.0 and using its new payment recovery API. The CLI deliberately does not recover on its own (no wallet polling, no automatic retries) - the calling agent must have the context of what happened, so the CLI exits immediately with a structured error carrying everything needed to recover: - If pay_invoice fails (e.g. an NWC reply timeout), the payment may still settle. The error output includes a paymentRecovery object with the payment hash, the pendingPayment token, and instructions: check lookup-invoice, then re-run the fetch with the new --resume flag to get the content without paying again, or retry normally if the wallet confirms the payment failed. - If the request after a successful payment fails (network error or non-OK response), the error surfaces the full payment metadata (amountSat, preimage, credentials) with instructions to re-run with --credentials instead of re-paying. - v9.0.0 also renames the payment output fields to unit-explicit amountSat / feesPaidMsat, so agents no longer misread the msat fee as sats (#58). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe fetch command now supports structured payment recovery errors and a ChangesFetch payment recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant fetchCommand
participant fetch402
participant NWCClient
User->>fetchCommand: provide URL and optional --resume
fetchCommand->>fetch402: pass validated parameters
fetch402->>NWCClient: pay invoice when no resume credentials exist
NWCClient-->>fetch402: payment result or error
fetch402-->>fetchCommand: content or structured recovery error
fetchCommand-->>User: output content or recovery instructions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/tools/lightning/fetch.ts (1)
65-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: give
resultan explicit type instead oflet result;.
let result;infersany, soresult.text(),result.ok, andresult.paymentbelow lose type-checking against thefetch402Libreturn type. Consider typing it (e.g.let result: Awaited<ReturnType<typeof fetch402Lib>>;) to keep the payment-metadata access type-safe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/lightning/fetch.ts` around lines 65 - 78, Update the result variable in the fetch402Lib flow to use an explicit type derived from fetch402Lib’s awaited return value, preserving type-checking for result.text(), result.ok, and result.payment while leaving the existing error handling unchanged.src/commands/fetch.ts (1)
172-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
Option.conflicts()for--credentialsand--resumeCommander can enforce this at parse time with a standard usage error instead of a manual runtime check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/fetch.ts` around lines 172 - 178, Replace the manual mutual-exclusion check in the fetch command’s option definitions with Commander’s Option.conflicts() configuration for --credentials and --resume. Remove the corresponding runtime throw while preserving the existing option behavior and conflict message semantics through Commander’s standard parse-time validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/lightning/fetch.ts`:
- Around line 122-130: Update the recovery payload passed to paidRecoveryDetails
in the fetch flow to use the PaymentInfo field names amount and feesPaid
expected by `@getalby/lightning-tools` v9.0.0. Translate the invoice amount and
any fee value into those properties, removing the unsupported
amountSat/feesPaidMsat shape while preserving the existing payment, preimage,
credentials, and paymentHash values.
---
Nitpick comments:
In `@src/commands/fetch.ts`:
- Around line 172-178: Replace the manual mutual-exclusion check in the fetch
command’s option definitions with Commander’s Option.conflicts() configuration
for --credentials and --resume. Remove the corresponding runtime throw while
preserving the existing option behavior and conflict message semantics through
Commander’s standard parse-time validation.
In `@src/tools/lightning/fetch.ts`:
- Around line 65-78: Update the result variable in the fetch402Lib flow to use
an explicit type derived from fetch402Lib’s awaited return value, preserving
type-checking for result.text(), result.ok, and result.payment while leaving the
existing error handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58c4d2a5-b795-423d-88a9-42ecbf25152d
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
package.jsonsrc/commands/fetch.tssrc/test/fetch-402-recovery.test.tssrc/tools/lightning/fetch.tssrc/utils.ts
…ted payments Consumes the renamed Fetch402InterruptedError and its new amountSat / feesPaidMsat fields, so the paymentRecovery error output now includes the routing fee instead of omitting it, and the amount no longer needs to be re-derived from the invoice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes #55, fixes #58.
Updates
@getalby/lightning-toolsto v9.0.1 (getAlby/js-lightning-tools#332, #333, #334) and uses its new payment recovery API infetch.Design
The CLI deliberately does not recover on its own - no wallet polling, no automatic retries. The calling agent must have the context of what happened; a CLI silently waiting on an in-flight payment would just look hung. Instead, when a payment is interrupted the CLI exits immediately with a structured error carrying everything needed to recover without ever paying the same invoice twice.
Changes
pay_invoicefails (e.g. an NWC reply timeout - the #55 scenario): the payment may still settle, so retrying blindly could double-pay. The error output now includes:{ "error": "payment did not complete: reply timeout: ... The payment may still settle - do NOT retry this fetch yet.", "paymentRecovery": { "status": "unknown", "paymentHash": "...", "pendingPayment": { "scheme": "l402", "header": "Authorization", "token": "...", "authScheme": "L402" }, "instructions": "... First run: lookup-invoice --payment-hash <hash> - if it returns a preimage, the payment settled: re-run the same fetch adding: --resume '{...}' to get the content without paying again. If it shows state \"failed\", no funds moved and it is safe to re-run the fetch normally. ..." } }New
--resumeflag: completes an interrupted payment in a later invocation - pass thependingPaymentfrom the error plus the preimage recovered vialookup-invoice; the library rebuilds the credential and never pays again. Mutually exclusive with--credentials.Request after a successful payment fails (network error or non-OK response): the error surfaces the full payment metadata (
amountSat,feesPaidMsat,preimage,credentials, andpaymentHash) with instructions to re-run with--credentialsinstead of re-paying - so paid content is never lost.#58: v9.0.0 renames the
paymentoutput fields to unit-explicitamountSat/feesPaidMsat, so agents no longer misread the millisat fee as sats. Help text updated to match.Tests
src/test/fetch-402-recovery.test.tscovers: recovery info on pay_invoice failure (asserting the CLI makes exactly one payment attempt and no retries), credential surfacing on post-payment network failure and non-OK response, no recovery details on unpaid non-OK responses, and--resume/--credentialspassthrough without paying. Full suite: 117 passed, 2 skipped.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes